runtime: fold venue adapters into the restart and poison sweeps#445
runtime: fold venue adapters into the restart and poison sweeps#445mfw78 wants to merge 1 commit into
Conversation
A trap in a routed adapter call marks a shared Liveness dead; the dispatch-time sweeps restart the provider after the module backoff policy and quarantine a crash-looper per the poison policy. A dead venue resolves to unavailable, distinct from unknown-venue, and adapter_alive_count reports live routability. The flaky-venue fixture drives the trap-to-recovery and poison paths end to end.
lgahdl
left a comment
lgahdl
left a comment
There was a problem hiding this comment.
Solid end-to-end feature — verified the highest-risk correctness questions are clean: the shared Liveness handle is correctly carried through reinstall_provider's swap (no window where an unconfirmed instance gets routed to), Unavailable/UnknownVenue are both exercised by tests, poison quarantine permanently gates the restart branch, and adapter_alive_count reads liveness directly so it can't drift from the sweep's cached state. Three things worth a look:
| // failure here is unreachable; skip defensively regardless. | ||
| // A dead venue fails to resolve; its watch stays for the | ||
| // cadence after the sweep restarts the adapter. | ||
| let Ok(slot) = self.resolve(&venue) else { |
There was a problem hiding this comment.
The comment says a dead venue's "watch stays for the cadence after the sweep restarts" — true, the entry itself isn't evicted here. But since resolve() failing means this continues before ever reaching record_polled_status (where expires_at gets refreshed, per #438), a venue that takes multiple backoff cycles to recover never gets its watch-expiry deadline pushed out during the outage. If the outage outlasts the configured watch-expiry window, prune_expired will silently evict the pending watch for an intent whose venue just happens to still be mid-recovery — losing status tracking for that intent, not just delaying it. Worth either refreshing expires_at on a dead-venue skip too, or explicitly noting the interaction with the watch-expiry policy so it's a documented tradeoff rather than a surprise.
| { | ||
| provider.alive = false; | ||
| provider.failure_count = provider.failure_count.saturating_add(1); | ||
| let backoff = crate::runtime::restart_policy::backoff_for(provider.failure_count); |
There was a problem hiding this comment.
Providers get byte-identical backoff/poison tuning to modules (backoff_for/poison_policy are the same engine-wide policy shared with sweep_modules) despite a strictly larger blast radius — one dead venue stalls every module routing through it, not just its own workflow. A popular shared venue (e.g. a CoW adapter serving many modules) traps under load and gets the same 1s-then-doubling curve and poison threshold as a niche single-module adapter, with no faster-detect or wider-tolerance curve reflecting how many callers are blocked on it. Worth at least a comment on why uniform tuning is acceptable at current scale, or a per-provider policy override as a follow-up.
| /// liveness reports (backoff plus poison bookkeeping), then reinstall | ||
| /// dead, unpoisoned providers whose backoff has elapsed. Runs at the | ||
| /// head of every dispatch, beside the module restart sweep. | ||
| async fn sweep_providers(&mut self) { |
There was a problem hiding this comment.
sweep_providers is a hand-duplicated copy of sweep_modules's death-detection/backoff/restart control flow (not a shared generic loop over a common trait) — a future third actor kind needing sweep participation would need its own bespoke sweep_x/try_restart_x/LoadedX rather than slotting into existing infrastructure. Not blocking for this PR, but worth flagging before a third kind shows up: a Sweepable trait (is_alive/mark_checked/restart) would let sweep_modules/sweep_providers converge into one generic loop.
2 participants
What
Fold venue adapters (providers) into the supervisor's restart and poison-recovery sweeps, alongside modules. A trap in a routed adapter call marks a shared
Livenessdead; the sweep restarts the provider through the module backoff policy and quarantines a crash-looper through the poison policy. A dead venue now resolves toUnavailable, distinct fromUnknownVenue, andadapter_alive_counttracks live routability. Adds theflaky-venuefixture, whosesubmittraps on a poison chain-head sentinel and recovers once the head moves on, to drive the trap-to-recovery and poison paths end to end.Why
Adapters currently boot once and install but sit outside the sweeps, so a trapped adapter stays dead until the process restarts, and the router can only project the trap to an internal error. Folding adapters into the sweeps recovers them automatically and lets a strategy tell an unknown venue apart from a temporarily dead one.
Closes #378
Testing
dead_venue_is_unavailable_not_unknown,a_dead_incumbent_is_replaced_on_reinstallinhost/venue_registry.rs.e2e_trapped_adapter_is_swept_and_restarts,e2e_crash_looping_adapter_is_poisonedinsupervisor/tests.rs, over the newflaky-venuewasm fixture.cargo fmt --all --check,cargo check --workspace --all-features,cargo clippy --workspace --all-targets --all-features -- -D warnings,cargo nextest run --workspace --all-features,cargo test --doc.AI Assistance
Implemented with Claude Code under review.